ETag for static file server #2243
Conversation
Programmed feature yhirose#2242. First of, the static file server now sends HTTP response header "ETag". Following HTTP requests by the client which include HTTP request header "If-None-Match" are only served if the value for HTTP response header "ETag" is not included in the value of HTTP request header "If-None-Match", otherwise an HTTP response with status code 304 is served which includes the HTTP response header "ETag" again that would have been sent with a normal status code of 200. Useful resources: - https://http.dev/caching - https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/ETag - https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/If-None-Match - https://www.rfc-editor.org/rfc/rfc9110.html#name-304-not-modified
|
Please check for style and do adjustements. Please adjust the location of the test. I just put it at the end and the names should be adjusted too. Does |
|
Test "ServerTest.StaticFileRangeBigFile" causes a segmentation fault. Do you have an idea for the reason? |
Added more comments. Check for method to be GET or HEAD. Explicitly clear the body.
|
The solution was done with one line. A c-string did not have null termination. 100% tests passed, 0 tests failed out of 444
Total Test time (real) = 365.31 sec
The following tests did not run:
114 - BindServerTest.BindDualStack (Disabled)
390 - SSLClientServerTest.LargeDataTransfer (Disabled) |
|
The function This is just a thought and has no effect on on this pull request. Only the tests would be needed to be updated. |
There was a problem hiding this comment.
lgfm. But I suggest you to move etag generation algo from handle_file_request and add func like add_etag_enable() what set the algo. class Server (its user) should know about the extension. Explicit is better than implicit.
And second moment. On 7924 line you check etag.length() >= 2 and use then .at(0) and .at(1) that check bounds: .length() > 0 and .length() > 1, it is useless :) (ref: https://en.cppreference.com/w/cpp/string/basic_string/at.html).
These remarks do not affect logic, it is correct. And your work is great.
|
Thank you for the review. I am glad that this feature is well perceived. Regarding your first recommendation, do you suggest a switch (member variable on class From my current knowledge, One reason for disabling might be due to performance. Do I even understand you correctly? What do you think? Regarding your second recommendation, I know that I think the condition needs to be like before Normally, I would use |
2 participants
Programmed feature #2242. First off, the static file server now sends HTTP response header "ETag". Following HTTP requests by the client which include HTTP request header "If-None-Match" are only served if the value for HTTP response header "ETag" is not included in the value of HTTP request header "If-None-Match", otherwise an HTTP response with status code 304 is served which includes the HTTP response header "ETag" again that would have been sent with a normal status code of 200.
Useful resources: